Skip to content

Capturing CTRL+D (EOF) to exit the shell #432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 7, 2022

Conversation

eubnara
Copy link
Contributor

@eubnara eubnara commented May 29, 2022

  • support capturing CTRL+D to exit the shell
  • add some tests for CTRL+C, CTRL+D cases

@eubnara
Copy link
Contributor Author

eubnara commented May 29, 2022

With spring shell v1.2.0, ctrl+d exits the shell. I think this is the expected behavior.

@jvalkeal
Copy link
Contributor

It's strange that InteractiveShellRunnerTests hangs up with jdk 8/11 but not with 17.

import org.jline.utils.AttributedStyle;
import org.junit.jupiter.api.Test;
import org.springframework.shell.ExitRequest;
import static org.junit.jupiter.api.Assertions.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use assertj.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jvalkeal
Thanks for feedback!
I changed it to use assertj.

@eubnara
Copy link
Contributor Author

eubnara commented Jun 1, 2022

I currently use java 11.0.14 eclipse temurin installed by sdkman.

@jvalkeal
Copy link
Contributor

jvalkeal commented Jun 1, 2022

I think this would be a good change and it did work when I ran a shell. We just can't merge it in this state as it jams a build. I briefly tried to debug it and it hangs on a thread somewhere inside jline classes.

@jvalkeal
Copy link
Contributor

jvalkeal commented Jun 2, 2022

Great tests passes. I'll mark this to get merged.

@jvalkeal jvalkeal added this to the 2.1.0-M5 milestone Jun 2, 2022
@jvalkeal jvalkeal merged commit 38ce784 into spring-projects:main Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants